@gravitystorm Anything I can do to help with a review, or just waiting for time?

I've had a look at this again today.

• The end result looks fine in terms of the dash patterns, and I'm happy to see just one track colour instead of three
• The comp-op is unnecessary - src-over is the default when no comp-op is specified. Since you're specifying the opacity for the two ::background attachments (rather than the symbolizers) then you'll get the effect that we want, without specifying any comp-ops.
• I don't think that the bridgescase instances are necessary, I think it will be ok with the default instance
• There seems to be a lot of duplication between putting things into attachments and also having instances with the same name
• Is the ::line attachment necessary? If so (perhaps due to the ::background attachment?) then the instances probably aren't needed.
• I'm also concerned that the ::line and ::background attachments are going to mess up the zorder, since they effectively pull the tracks out of the normal zorder and draw them after everything else

It would be good to have comments from @math1985 too. Most of the questions above I can answer myself, next time I look at it in more detail.

Yes, I'm planning to have a look at it.

@pnorman thank you for the effort, it looks a lot better now, both in code and in output.

Some comments:

• Like @gravitystorm says, the ::line attachment is a problem. Because :line gets introduced later than ::fill, tracks are now rendered on top of everything. That's a problem, especially when tracks cross primary roads etc. Leaving out ::line requires leaving out ::background as well, because otherwise the background gets rendered on top of the line. I think if you use instead background/line-opacity: 0.4; and line/line-opacity: 0.8;, and remove the attachments, the result is visually indistinguishable. I realize this treats overlapping tracks slightly different, but I don't think it is a problem in practice (and if it is, then we have the same problem at the moment for footways etc.).
• Background of tunnels should have no opacity attribute (because whatever is below is guaranteed to be black).
• I agree that the bridgecase instance is not necessary and has no effect on rendering.
• In-line comments with // are not valid CSS and probably also not valid CartoCSS (although I don't know where to find a formal specification of CartoCSS). I think it would be better if we stick with /* */
• I would prefer to have definitions for all zoom-levels together, in other words, to have tracktype as outer selector and zoom as inner, but that's perhaps a matter of personal preference. See the other definitions (for example highway=primary) for an example.
• It's probably better to specify the sizes with variables, but this can be done later.
• The meaning of tracks without tracktype is not immediately clear from the rendering (dasharray: 5,4,2,4;), but I also don't see how else we can render them.
• In terms of rendering, I have the impression that the opacity is a bit too high. The tracks look rather whitish. This is especially visible for high-tracktype tracks on low zoom levels when they pass through forests.

If these issues are resolved, I would like to see this change request merged.

@pnorman Do you agree with my comments? Let me know if you need any help.

@pnorman Do you agree with my comments? Let me know if you need any help.

I'm not going to have time to get to this in the near future.

The meaning of tracks without tracktype is not immediately clear from the rendering

We're trying to show an "unknown" meaning

In terms of rendering, I have the impression that the opacity is a bit too high. The tracks look rather whitish. This is especially visible for high-tracktype tracks on low zoom levels when they pass through forests.

We might need to change opacity with zoom then, I increased it because the tracks were hard to see in my test area.

I'm not going to have time to get to this in the near future.

No problem, thank you for your work so far. I can take over (it is only minor changes that still need to be done). Is there a way to pull your branch and then create a PR for gravitystorm rather than for you?

the tracks were hard to see in my test area.

What was the background? I mainly looked at tracks through forests.

http://www.openstreetmap.org/#map=13/52.1059/-1.1447 was the general area. I just downloaded it from overpass and loaded it locally. I had the downloaded file open in JOSM too so I could find particular combinations (e.g. a track tunnel next to a track non-tunnel, both with tracktype). There are all tracktypes in the area, as well as many over bridges or in tunnels.

Part of the issue is there's many footways in the region, and the footways are stronger than the tracks. Of course, footway rendering does kind stick out excessively at low zoom (#243), so perhaps we shouldn't give that too much consideration, and revise the low-zoom footway rendering later.

Is there a way to pull your branch and then create a PR for gravitystorm rather than for you?

Yes. This should work

git remote add pnorman https://github.com/pnorman/osm2pgsql.git
git fetch pnorman
git checkout pnorman/track_rewrite2
git checkout -b track_rewrite3

Since there's been other changes, you probably also want to do git rebase master